Why is the PR here needed? I would be under the impression that GmsCore upstream already implements Maps. If not, would this PR not be suitable to upstream GmsCore?

MapContext hardcoded com.google.android.gms when creating the package context. When GmsCore is installed under a different package name (e.g. app.revanced.android.gms), this bound to the wrong package — or a disabled real Google Play Services install — so the bundled libmapbox-gl.so could not be extracted and the map failed to render (UnsatisfiedLinkError). It now resolves whichever GmsCore package is actually installed.

This makes sense for ReVanced, the patch is mandatory in our repository. Can you confirm that it is not already tackled by #308?

feat(maps): Fall back to key-less OpenStreetMap tiles when no map key is set.

This change seems to be intended to upstream. If upstream decides that the current behaviour is right, can upstream explain why they do not implement the same fallback mechanism

Thanks for the PR!

Thanks for the quick review - all three points are fair, and you are right this PR shouldn't live in the fork.

MapContext / package resolution: #308 already covers this, and more comprehensively. So this is redundant with #308.

Keyless OSM fallback: Agreed it's general rather than ReVanced-specific, so it would belong in microG upstream. But there's likely a deliberate reason upstream requires a key instead of shipping a free fallback: OpenStreetMap's Tile Usage Policy forbids using tile.openstreetmap.org from distributed apps, and the Esri/ArcGIS endpoint I used for satellite is the same. At GmsCore's scale a default keyless raster source would violate those terms and risk being blocked. I'll figure that out separately.

BitmapDescriptorFactory message: just a wrong string in an exception (CameraUpdateFactory → BitmapDescriptorFactory), no functional impact — not worth carrying on its own.

Given that, I'll close this. Thanks for the maps work in #308!

BitmapDescriptorFactory message: just a wrong string in an exception (CameraUpdateFactory → BitmapDescriptorFactory), no functional impact — not worth carrying on its own.

Yeah, it can be reported as a small issue to upstream, doesn't change functionality.

Regarding the Tile Usage Policy, we can ship a key in our releases, if that's intended, correct, and free.

This states its free for non-commercial free use, which is the case here https://www.maptiler.com/cloud/pricing/

Just to clarify, you are open to ship a maptiler key in the ReVanced fork releases?
Who would obtain this key?

ReVanced as the organization would, however since we are registered non-profit and the use of the key is also uncommercial, I suppose we qualify for a free key?

The pricing page does state that non-commertial usage is in the free tier, however, the free tier has limits. Instead of calculating if/when all the users would hit this, I think a better approach would be to expose a settings page in GmsCore (upstream) so the users can supply their own key for a map provider (currenly only Mapbox is implemented, but OSM/maptiler could easily be implemented very similiar to this PR).

And then the users can provide their own key, and choose their own map provider (Mapbox, OSM, maptiler, etc.)

This sounds worth pursuing to me. I would appreciate your input because you already have expertise in this codebase/community while I am just ramping up for the first time.

I think this would breach the scope of this fork, so again this would at max be something upstream should consider. The limit I can see is regarding some storage? What is that about?

If user supplied keys are the only option however, then we'd have to look if its in upstreams scope to implement user supplied keys, if not then it would become subject to our fork under the basis of their reason

I was not clear in my previous message. The whole proposal was referring to upstream (ofc).

Let me first state the limitations I see:
MapTiler offers 100k API requests. That means 100k renders from all users of different parts of the map. The (wrong) question to ask here is how many users does ReVanced have.

OSM requires that large-volume clients set up their own tile servers. But I am still not sure how individual users are classified here; it could be the case that they could be considered separate private use. Either way OSM would be an upstream solution.

BYOK is also an upstream solution.

Mapbox support is already present in upstream, but its key is a compile-time constant which would need to be made configurable in runtime. Otherwise, I would need to set-up my own build pipeline which supplies my own key (and my own apk signature).

I still haven't arrived at a strong opinion but Maptiler looks unappealing to me.

Perhaps BYOK with both Mapbox and OSM (if it even accepts a key) is the right way forward.

Let me know your thoughts :)

I strongly tend to BYOK as well. We cant draw any assumptions on user count, and its well expected to exceed the limits/be subject to trolling/key abuse, so yes, BYOK seems sensible. Does upstream provide a key for their releases? What qualified them and what are they using? We can reach out to them in this regard.

Upstream does not provide a key. I guess people have their own builds setup. But we can surely try make progress on either (BYOK or OSM)-for-all, with the upstream apk signature

Upstream does not provide a key

Is this assumed or have you checked the release files? It would be weird if upstream intends building your own distribution instead of a runtime BYOK or shipping a key with their release.

I checked. Config-time is null. Runtime is not configurable :)

I did test a build that used the mapcontext handling of 308 and the keyless patches of 370 and it does appear to work fine.

I was under the impression that maps wouldnt work on keyless builds. If it still does, what makes the API key necessary?
If the key is necessary, this kind of fallback mechanism might actually be indeed out of scope of upstream given the presumed reason "builds should ship a key" as they also do with Stadia. Since upstream does ship a key, it would be interesting to know how they do. Do the pay? Do they use a non-commercial key? Do they have a custom agreement? Can we replicate this deployment?

If we were to implement something like this I think we should include OSM (And Esri?) attribution, because right now I only see it display “MapLibre Maps SDK for Android”.

Wouldnt that be a concern of upstream? Upstream release builds do not use OSM, but given the code is intended to be built in either flavor, the flavor should also control the string you mentioned.

@zappybiby did your comment disappear?

Sorry for confusion, a ton of comments loaded after my comment posted.

A key is needed for microG’s normal Stadia/Mapbox style path. If no Stadia or Mapbox key is provided, #370 can still render a map by switching to a raster based tile map with OSM.

So the question is which provider would ReVanced want to use or to keep it BYOK. It appears MicroG currently provides a Stadia key from what I found.

I looked at the history:

• v0.2.28.231657 from May 2023: official APK had a non-empty Mapbox key and no Stadia key.
• Stadia key support was added in 2023 by commit c666326 titled Inject stadia API key if set.
• v0.2.29.233013 from September 2023: official APK had empty MAPBOX_KEY and non-empty STADIA_KEY.
• v0.3.0.233515 and latest v0.3.15.250932: same pattern, empty Mapbox key, non-empty Stadia key.

In microG, keys can be configured with MAPBOX_KEY in local.properties, or
MAPBOX_VECTOR_TILES_KEY as an environment variable. Similar for Stadia.

So presamably their release environment sets STADIA_API_KEY. It then gets baked into BuildConfig.STADIA_KEY

Don't know their arrangement with stadia.